[CFX-5789] [CFX-5790] `ToolRegistry` & Surface environment-aware failure messaging by taras-pokornyy · Pull Request #577 · datarobot-oss/cli

taras-pokornyy · 2026-06-16T15:41:39Z

RATIONALE

When dr deps install runs a tool's install command and it fails, the previous error was a bare exit-code line with no guidance on what to try next. This change wires up the existing ToolRegistry data to produce actionable, environment-aware tips when an install command fails — "You have pyenv — try: pip install uv" — and falls back to the canonical install command (e.g. curl | sh) when no manager is detected. Permission-denied failures (exit code 126 or matching stderr text) get an OS-specific "re-run with sudo / as Administrator" message instead.

CHANGES

PR Automation

Comment-Commands: Trigger CI by commenting on the PR:

/trigger-smoke-test or /trigger-test-smoke - Run smoke tests
/trigger-install-test or /trigger-test-install - Run installation tests

Labels: Apply labels to trigger workflows:

run-smoke-tests or go - Run smoke tests on demand (only works for non-forked PRs)

Important

For Forked PRs: The run-smoke-tests label won't work. A required Smoke Tests check will block merge until a maintainer acts:

A maintainer uses /approve-smoke-tests to run smoke tests (results will set the check)
A maintainer uses /skip-smoke-tests to bypass the check without running tests

Please comment requesting a maintainer review if you need smoke tests to run.

Note

Low Risk
UX and messaging only around failed installs; success path unchanged. Suggestions are advisory shell commands, not auto-executed.

Overview
When dr deps install fails, users now get structured output on the writer (not only a minimal error): what was tried, optional tips, retry command, and docs URL. InstallPrerequisites tees install output into a buffer so failures can be classified and messaged.

A new ToolRegistry maps tools (python, uv, node, pulumi, task, git) to ordered install strategies (pyenv/brew/asdf/winget/etc.) plus fallbacks. DetectEnvironment probes PATH and NVM layout; selectInstallStrategy picks the first matching manager, skipping the manager that just failed. buildInstallFailureMsg / buildInstallTip add OS-specific guidance for permission errors (exit 126 / stderr heuristics) vs alternate install commands (e.g. “You have pyenv — try: pip install uv”). Returned errors are shortened; detail lives in stdout.

Broad unit coverage was added for registry, strategy selection, and failure messaging.

^{Reviewed by Cursor Bugbot for commit 6c10ba7. Configure here.}

cursor · 2026-06-16T15:44:51Z

+
+			fmt.Fprint(w, msg)
+
+			return installed, fmt.Errorf("install failed for %q (exit code %d)", prerequisite.Name, exitCode)


Start flow hides install failures

High Severity

Detailed install failure text is written only to the io.Writer, while the returned error is a short exit-code message. The quickstart TUI installs deps into a buffer and on failure displays only m.err, so users lose tips, the tried command, and streamed command output that this change added.

^{Triggered by project rule: Bugbot Rules for DataRobot CLI}

^{Reviewed by Cursor Bugbot for commit 29b422a. Configure here.}

taras-pokornyy · 2026-06-16T17:03:19Z

@cursor review

cursor

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 6c10ba7. Configure here.}

cursor · 2026-06-16T17:07:08Z

+	// Unix-like systems (Linux & macOS) return 126 when a file lacks execute permissions
+	if (runtime.GOOS == "linux" || runtime.GOOS == "darwin") && exitCode == 126 {
+		return true
+	}


Exit 126 ignored on Windows

Medium Severity

Permission-denied handling treats exit code 126 only when runtime.GOOS is linux or darwin. Installs still run via sh -c on Windows (e.g. Git Bash), where 126 can mean “not executable,” but without matching stderr text the failure is not classified as permission denied and users miss the Administrator-oriented tip.

Additional Locations (1)

internal/dependencies/installer_test.go#L342-L344

^{Triggered by project rule: Bugbot Rules for DataRobot CLI}

^{Reviewed by Cursor Bugbot for commit 6c10ba7. Configure here.}

c-h-russell-walker · 2026-06-17T18:11:28Z

+func (fs FallbackStrategy) getStrategyTip(goos string) string {
+	cmds := fs.Commands
+
+	if goos == "windows" && len(fs.CommandsWindows) > 0 {


nit here:

Do we have reusable values in this repo. so we're not using magic strings for things like "windows" and likely "darwin" and "linux" ?

"windows", "darwin", and "linux" are all potential values of runtime.GOOS, which is a build-time constant.

https://pkg.go.dev/internal/goos#GOOS
https://cs.opensource.google/go/go/+/refs/tags/go1.26.4:src/internal/goos/zgoos_linux.go;l=7

I am not recommending this because I'm not convinced it's worth it here but one way to approach this would be to conditionally compile the fallbackstrategy code using go build. (You can extend packages based on runtime.GOOS and runtime.GOARCH.) A working example of this can be found in internal/workload where Wojciech has implemented OS-specific diskspace implementations.

https://github.com/datarobot-oss/cli/blob/main/internal/workload/sync/diskspace_unix.go

https://github.com/datarobot-oss/cli/blob/main/internal/workload/sync/diskspace_windows.go

What MIGHT be worth doing is something like this, where we enumerate the OS we actually support.

type OS string const ( OSDarwin OS = "darwin" OSLinux OS = "linux" OSWindows OS = "windows" ) func IsWindows(os OS) bool { return os == OSWindows } func IsUnixLike(os OS) bool { return os == OSDarwin || os == OSLinux }

This is all just informational though, since I know @taras-pokornyy you want a bigger discussion on how we can scale this registry.

I think it is a good idea to have constants for GOOS instead of magic strings but not in the scope of this PR 🤷‍♂️

c-h-russell-walker · 2026-06-17T18:13:28Z

+		return ""
+
+	case 1:
+		return "  Try: " + cmds[0]


So wasn't sure where to put this.

Curious what you and @ajalon1 and others think.

I like your usage of \t elsewhere and wonder if that would be better to use than the many places you're using double spaces??

Suggested change

return " Try: " + cmds[0]

return "\tTry: " + cmds[0]

To me it's:

less likely to accidentally have one or three spaces at some point as a UI bug

easier to read in the code as opposed to empty spaces - maybe that's just me though

I think it'd be good to stick to a single convention, but that becomes a renderer / UI issue and I think we can do this better with a more structured formatting approach than just \t. text/tabwriter is a helpful package if we want to go that route.

That's a good point.
First level indention was two spaces. Next level indention was "\t" (It is much longer).

Created a const TAB which is two spaces and used that for indentions.
First level indention is just TAB
Next level indention is TAB+TAB

Commit : dc1017f

c-h-russell-walker

Looks great - made some nit comments. And thanks for the demo today during our meeting.

ajalon1

This LGTM as is. The registry could use some more work, but that is out of scope.

The one thing I would suggest addressing is what happens with strategy.WithVersion(""). You can either do that here or in a followup.

ajalon1 · 2026-06-18T03:57:37Z

+func (fs FallbackStrategy) getStrategyTip(goos string) string {
+	cmds := fs.Commands
+
+	if goos == "windows" && len(fs.CommandsWindows) > 0 {


"windows", "darwin", and "linux" are all potential values of runtime.GOOS, which is a build-time constant.

https://pkg.go.dev/internal/goos#GOOS
https://cs.opensource.google/go/go/+/refs/tags/go1.26.4:src/internal/goos/zgoos_linux.go;l=7

I am not recommending this because I'm not convinced it's worth it here but one way to approach this would be to conditionally compile the fallbackstrategy code using go build. (You can extend packages based on runtime.GOOS and runtime.GOARCH.) A working example of this can be found in internal/workload where Wojciech has implemented OS-specific diskspace implementations.

https://github.com/datarobot-oss/cli/blob/main/internal/workload/sync/diskspace_unix.go

https://github.com/datarobot-oss/cli/blob/main/internal/workload/sync/diskspace_windows.go

What MIGHT be worth doing is something like this, where we enumerate the OS we actually support.

type OS string const ( OSDarwin OS = "darwin" OSLinux OS = "linux" OSWindows OS = "windows" ) func IsWindows(os OS) bool { return os == OSWindows } func IsUnixLike(os OS) bool { return os == OSDarwin || os == OSLinux }

ajalon1 · 2026-06-18T04:04:58Z

+func (fs FallbackStrategy) getStrategyTip(goos string) string {
+	cmds := fs.Commands
+
+	if goos == "windows" && len(fs.CommandsWindows) > 0 {


This is all just informational though, since I know @taras-pokornyy you want a bigger discussion on how we can scale this registry.

ajalon1 · 2026-06-18T04:18:01Z

+		return ""
+
+	case 1:
+		return "  Try: " + cmds[0]


I think it'd be good to stick to a single convention, but that becomes a renderer / UI issue and I think we can do this better with a more structured formatting approach than just \t. text/tabwriter is a helpful package if we want to go that route.

taras-pokornyy · 2026-06-18T18:00:18Z

Hey @c-h-russell-walker , @ajalon1 !
Thank you for reviews and comments. I really appreciate that 😀
I believe I have fixed all the issues except GOOS magic strings.

Going to merge this guy tonight.
Please let me know in case of anything 😉

ajalon1

Still LGTM pending smoke test results

ajalon1 · 2026-06-18T18:03:31Z

nice, good test

taras-pokornyy · 2026-06-18T18:29:09Z

/approve-smoke-tests

github-actions · 2026-06-18T18:29:22Z

🔐 Fork PR smoke tests triggered by @taras-pokornyy

⚠️ Security Notice: This will run tests with access to repository secrets.

What happens next:

Security scans will run automatically (Trivy, gosec)
If security scans pass, smoke tests will run
Results will be posted as PR comments

⚠️ Important: Review the PR code carefully before approving!

github-actions · 2026-06-18T18:32:16Z

🔐 Fork smoke tests started by maintainer

⏳ Security scans passed. Running smoke tests...

Commit: 43727f4adb872ae30a323dd599fb5ae28c49c5e5
View run

github-actions · 2026-06-18T18:38:37Z

✅ All smoke tests passed! (Fork PR)

✅ Security Scan: success
✅ Linux: success
✅ Windows: success

View run details

taras-pokornyy added 2 commits June 15, 2026 16:37

[CFX-5789] TOOL_REGISTRY

4d5d244

[CFX-5789][CFX-5790] Surface environment-aware failure messaging

29b422a

taras-pokornyy self-assigned this Jun 16, 2026

cursor Bot reviewed Jun 16, 2026

View reviewed changes

taras-pokornyy added 3 commits June 16, 2026 19:10

Fix goos once permission denied

5786675

Add URL for manual unstallation

42b40d6

Fix lint

6c10ba7

cursor Bot reviewed Jun 16, 2026

View reviewed changes

taras-pokornyy requested a review from a team June 16, 2026 17:32

[CFX-5789] Handle MinimumVersion in ToolRegistry

3742576

c-h-russell-walker reviewed Jun 17, 2026

View reviewed changes

Comment thread internal/dependencies/registry.go

c-h-russell-walker reviewed Jun 17, 2026

View reviewed changes

c-h-russell-walker approved these changes Jun 17, 2026

View reviewed changes

ajalon1 approved these changes Jun 18, 2026

View reviewed changes

taras-pokornyy added 3 commits June 18, 2026 19:55

Fix getenv -> os.os.UserHomeDir()

ff5cca2

Add DefaultVersion to Strategies

e1fec45

Fix indents

dc1017f

ajalon1 approved these changes Jun 18, 2026

View reviewed changes

Comment thread internal/dependencies/installer_test.go

ajalon1 Jun 18, 2026

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, good test

taras-pokornyy added 2 commits June 18, 2026 21:12

Extra line

a4cf6c2

Trigger tests

43727f4

c-h-russell-walker approved these changes Jun 18, 2026

View reviewed changes

taras-pokornyy merged commit 3dedc6a into datarobot-oss:main Jun 18, 2026
31 of 32 checks passed


		fmt.Fprint(w, msg)

		return installed, fmt.Errorf("install failed for %q (exit code %d)", prerequisite.Name, exitCode)

Uh oh!

Conversation

taras-pokornyy commented Jun 16, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

RATIONALE

CHANGES

PR Automation

Uh oh!

cursor Bot Jun 16, 2026

Choose a reason for hiding this comment

Start flow hides install failures

Uh oh!

Uh oh!

Uh oh!

taras-pokornyy commented Jun 16, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cursor Bot Jun 16, 2026

Choose a reason for hiding this comment

Exit 126 ignored on Windows

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

c-h-russell-walker left a comment

Choose a reason for hiding this comment

Uh oh!

ajalon1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taras-pokornyy commented Jun 18, 2026

Uh oh!

ajalon1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taras-pokornyy commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

taras-pokornyy commented Jun 16, 2026 •

edited by cursor Bot

Loading